Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: port media/codecs to CMake #1143

Merged
merged 1 commit into from
Dec 16, 2022
Merged

feat: port media/codecs to CMake #1143

merged 1 commit into from
Dec 16, 2022

Conversation

cadubentzen
Copy link
Contributor

@cadubentzen cadubentzen commented Nov 21, 2022 •
edited
Loading

  • Compile media/codecs with CMake
  • Compile tests for media/codecs with CMake
  • Pass tests for media/codecs with CMake

Also:

  • Use the same clang-format version as the CI here

Related to #1047

@google-cla
Copy link

google-cla bot commented Nov 21, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@cadubentzen cadubentzen changed the title WIP: port media/codecs to CMake feat: port media/codecs to CMake Nov 21, 2022
@cadubentzen cadubentzen mentioned this pull request Nov 22, 2022
Merged
2 tasks
joeyparrish
joeyparrish requested changes Nov 30, 2022
View reviewed changes
packager/media/codecs/CMakeLists.txt Outdated Show resolved Hide resolved
packager/utils/bytes_to_string_view.h Outdated Show resolved Hide resolved
packager/utils/bytes_to_string_view.h Show resolved Hide resolved
@joeyparrish joeyparrish mentioned this pull request Nov 30, 2022
Merged
@zdanek
Copy link
Contributor

zdanek commented Dec 8, 2022

@cadubentzen can we go with this PR? What is blocking us?
I depend on it in two PRs :)

@cadubentzen
Copy link
Contributor Author

@cadubentzen can we go with this PR? What is blocking us? I depend on it in two PRs :)

Hi @zdanek sorry for the delay here. At this point, only the corporate CLA is blocking it. I have already pinged folks internally to double-check it but unfortunately, it's not solved yet and I can't really speed it up :(

@zdanek
Copy link
Contributor

zdanek commented Dec 9, 2022 •
edited
Loading

only the corporate CLA is blocking it

that's why I'm here as an individual playing at my own rules in spare time :)

But, yeah, I do understand. I still have a lot to do in mpd.
Cheers.

@cadubentzen cadubentzen mentioned this pull request Dec 14, 2022
Closed
3 tasks
@joeyparrish
Copy link
Member

@cadubentzen, the CLA bot is happy with you, so that's good enough for me. Can you fix the copyright headers (or let me know that you don't mind me doing it)? I'm ready to merge this once that is resolved.

@cadubentzen
Copy link
Contributor Author

cadubentzen commented Dec 15, 2022 •
edited
Loading

@cadubentzen, the CLA bot is happy with you, so that's good enough for me. Can you fix the copyright headers (or let me know that you don't mind me doing it)? I'm ready to merge this once that is resolved.

@joeyparrish Thanks, I just fixed the copyright headers. Could you please re-trigger CI here?

@cadubentzen cadubentzen marked this pull request as ready for review December 15, 2022 17:01
@joeyparrish
Copy link
Member

Looks like there are some build errors. At least one was a missing header. Can you please take a look?

@joeyparrish
Copy link
Member 

D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(192,17): error C2220: the following warning is treated as an error [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(192,17): warning C4459: declaration of 'kSps' hides global declaration [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(17,15): message : see declaration of 'shaka::media::`anonymous-namespace'::kSps' [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(212,17): warning C4459: declaration of 'kSps' hides global declaration [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(17,15): message : see declaration of 'shaka::media::`anonymous-namespace'::kSps' [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(233,17): warning C4459: declaration of 'kSps' hides global declaration [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(17,15): message : see declaration of 'shaka::media::`anonymous-namespace'::kSps' [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(259,17): warning C4459: declaration of 'kSps' hides global declaration [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(17,15): message : see declaration of 'shaka::media::`anonymous-namespace'::kSps' [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
  h265_byte_to_unit_stream_converter_unittest.cc
  h265_parser_unittest.cc
  h26x_bit_reader_unittest.cc
  hevc_decoder_configuration_record_unittest.cc
  hls_audio_util_unittest.cc
  nal_unit_to_byte_stream_converter_unittest.cc
D:\a\shaka-packager\shaka-packager\.\packager/media/formats/mp4/box_definitions_comparison.h(350,51): error C2220: the following warning is treated as an error [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\.\packager/media/formats/mp4/box_definitions_comparison.h(350,51): warning C4100: 'rhs': unreferenced formal parameter [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\.\packager/media/formats/mp4/box_definitions_comparison.h(349,51): warning C4100: 'lhs': unreferenced formal parameter [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\.\packager/media/formats/mp4/box_definitions_comparison.h(491,73): warning C4100: 'rhs': unreferenced formal parameter [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\.\packager/media/formats/mp4/box_definitions_comparison.h(491,46): warning C4100: 'lhs': unreferenced formal parameter [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]

@cadubentzen cadubentzen marked this pull request as draft December 15, 2022 21:22
@cadubentzen
Copy link
Contributor Author

@joeyparrish I'm also now checking in parallel with the CI builds at my fork. Will ping you once the build is green there so you can retry it here 🙂

@joeyparrish
Copy link
Member

See also #1157, which fixes the Linux release builds. (If you'll review it, I'll go ahead and merge it today.)

@cadubentzen cadubentzen marked this pull request as ready for review December 15, 2022 22:31
@cadubentzen
Copy link
Contributor Author

@joeyparrish it should be good to go now 🙂 Got a green CI here.

@joeyparrish
Copy link
Member

Running it again now. BTW, I am working on ways to speed up CI builds so you can have a tighter feedback loop in situations like this. It might take some time for me to get it right, but I'm working on it.

@joeyparrish joeyparrish merged commit e9bf0c6 into shaka-project:cmake Dec 16, 2022
@cadubentzen cadubentzen deleted the cmake-media-codecs branch December 16, 2022 11:03
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@joeyparrish joeyparrish joeyparrish approved these changes

Assignees
No one assigned
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cadubentzen @zdanek @joeyparrish